Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add hover buttons to recipe cards #1577

Merged
merged 10 commits into from
May 10, 2024
Merged

Conversation

jonathonherbert
Copy link
Contributor

@jonathonherbert jonathonherbert commented May 7, 2024

What's changed?

Add working hover buttons to recipe cards. The 'view' button takes you to the source article for the recipe (canonicalArticle).

I didn't add Ophan – but perhaps that could also refer to the article?

hover-actions

Implementation notes

This includes a refactor of the hover states to factor out the code that reasons about URLs into the parts of the app that care about them. The problem was that this code didn't know about recipes, and it didn't feel appropriate for a simple hover button to know so much about parts of the app that we know change at a different cadence. The new API is hopefully a little simpler and more idiomatic – opinions welcome.

Tested locally with Fronts and Editions cards of article and snaplink, as well as our new recipe cards.

Checklist

General

  • 🤖 Relevant tests added
  • ✅ CI checks / tests run locally
  • 🔍 Checked on CODE

Client

  • 🚫 No obvious console errors on the client (i.e. React dev mode errors)
  • 🎛️ No regressions with existing user interactions (i.e. all existing buttons, inputs etc. work)
  • 📷 Screenshots / GIFs of relevant UI changes included

@jonathonherbert jonathonherbert requested a review from a team as a code owner May 7, 2024 16:18
@jonathonherbert jonathonherbert changed the base branch from main to jsh-db/add-recipe-card-to-collection May 8, 2024 09:41
@jonathonherbert jonathonherbert force-pushed the jsh-db/add-recipe-card-to-collection branch 2 times, most recently from b2ba058 to a7de1b7 Compare May 8, 2024 09:53
Copy link
Contributor

@Divs-B Divs-B left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, working as expected. Quick scan of code looks good too 👍

Divs-B added a commit that referenced this pull request May 9, 2024
@jonathonherbert jonathonherbert force-pushed the jsh-db/add-recipe-card-to-collection branch from a7de1b7 to 95115a4 Compare May 9, 2024 09:39
@Divs-B Divs-B mentioned this pull request May 9, 2024
6 tasks
Base automatically changed from jsh-db/add-recipe-card-to-collection to main May 9, 2024 14:59
@jonathonherbert jonathonherbert force-pushed the jsh/refactor-hover-buttons branch 8 times, most recently from a041caf to aeccbbb Compare May 9, 2024 16:00
@jonathonherbert jonathonherbert changed the base branch from main to db/add-chef May 9, 2024 16:45
Base automatically changed from db/add-chef to main May 10, 2024 10:14
@jonathonherbert jonathonherbert merged commit c2e42d6 into main May 10, 2024
@jonathonherbert jonathonherbert deleted the jsh/refactor-hover-buttons branch May 10, 2024 10:15
@prout-bot
Copy link

Seen on PROD (merged by @jonathonherbert 11 minutes and 4 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants